Just to check, this will go out as part of v1.2, right?

@matthew-white Yes, it will, but I'll wait a few days before merging. I want to test the PR a little bit more.

OK, sounds good! Since merging deploys immediately to production, I can also merge later when we release v1.2.

I can also merge later when we release v1.2.

I think that makes the most sense and we should make sure that there is documentation in place at that time.

It seems like the compose file allows you to specify a default value

YAY. I was very sad when @yanokwa said that was not possible.

During today's call, @issa-tseng said that this PR is ✅, so I'll plan to merge this when we release v1.2. I think that @issa-tseng is going to add instructions to the docs for upgrading to v1.2 related to moving .env. We can also highlight those instructions in the release announcement on the forum. @yanokwa, are there any other to-dos for this PR? Is there more testing that you want to do?

I'm happy with merging. Thanks for all the great feedback, team!

i wonder if we should enforce that if upstream is set, we require incoming requests to carry X-Forwarded-Proto: https.

@issa-tseng Not sure the implications. In my testing setup, the proxy_pass happens over HTTP (and the Central installs are also listening via HTTP on port 80). If you can send me the config you are proposing, I'm glad to test it.

server {
    server_name odk-central.example.com;
    location / {
            proxy_pass http://0.0.0.0:8080;
    }
    listen [::]:443 ssl;
    listen 443 ssl;
    ssl_certificate /etc/ssl/certs/server.crt;
    ssl_certificate_key /etc/ssl/certs/server.key;
    ssl_trusted_certificate /etc/ssl/certs/server.pem;
}
server {
    server_name odk-central-testing.example.com;
    location / {
            proxy_pass http://0.0.0.0:9080;
    }
    listen [::]:443 ssl;
    listen 443 ssl;
    ssl_certificate /etc/ssl/certs/server.crt;
    ssl_certificate_key /etc/ssl/certs/server.key;
    ssl_trusted_certificate /etc/ssl/certs/server.pem;
}
server {
    if ($host = odk-central.example.com) {
        return 301 https://$host$request_uri;
    }
    if ($host = odk-central-testing.example.com) {
        return 301 https://$host$request_uri;
    }
    listen 80 ;
    listen [::]:80 ;
    server_name odk-central.example.com odk-central-testing.example.com;
    return 404;
}

i'm not, yet. i'm wondering if that kind of enforcement would even have support amongst y'all.

Doesn't seem like a priority to me. Most of Central won't work properly without SSL anyway, right? I'd rather we just put "Central will not work properly without HTTPS" in the docs and if people want to try to get around that, that's their prerogative.

I've force pushed a change to set X-Forwarded-Proto https when you set SSL_TYPE=upstream.

• Pros: This is that it matches the intent of someone who sets upstream without them
• Cons: It's not safe? Maybe we remove the header entirely?

I took a quick look at @matthew-white's suggestion that we only set if the header if it isn't set already, but this suggests it isn't possible.

I think as long as we're operating under the assumption that the upstream SSL is working, this seems like a reasonable change. It'd be great to hear what @issa-tseng thinks.

Maybe we remove the header entirely?

I believe that we do need the header for cookie auth to work.

I took a quick look at @matthew-white's suggestion that we only set if the header if it isn't set already, but this suggests it isn't possible.

I actually think the second answer is promising. It'd be something like:

map $http_x_forwarded_proto $forwarded_proto {
  ~. $http_x_forwarded_proto;
  default $scheme;
}

# ...

proxy_set_header X-Forwarded-Proto $forwarded_proto;

I don't think map can go inside a server block, but maybe it can go before the server block in the Central config file? (Not sure.)

I just noticed that this has a similar suggestion (though without the regular expression).

The suggestion to map works as far as the syntax, but it doesn't solve the redirect problem.